Fix substring matches in WWW-Authenticate parsing by Whning0513 · Pull Request #3041 · modelcontextprotocol/python-sdk

Whning0513 · 2026-07-01T13:03:10Z

extract_field_from_www_auth currently matches field_name as a substring of another auth-param name, so fields like scope can be shadowed by error_scope and resource_metadata can be shadowed by x_resource_metadata.

This change parses comma-delimited auth parameters without splitting quoted values, preserves the generic helper's support for non-Bearer schemes, and scopes the RFC-specific scope and resource_metadata helpers to the first Bearer challenge. It also handles quoted-pair escapes and optional whitespace around =.

Tested with:

uv run --frozen pytest tests/client/test_auth.py -q (143 passed, 1 xfailed)
targeted branch coverage for src/mcp/client/auth/utils.py (100% statements and branches)
uv run --frozen ruff check src/mcp/client/auth/utils.py tests/client/test_auth.py
uv run --frozen ruff format --check src/mcp/client/auth/utils.py tests/client/test_auth.py
uv run --frozen pyright src/mcp/client/auth/utils.py tests/client/test_auth.py

AI assistance was used for parser edge-case analysis and test review; I reviewed and validated the resulting changes.

Copilot

Pull request overview

This PR fixes a correctness issue in extract_field_from_www_auth where an auth-param name could be matched as a substring of a different param name (e.g., scope incorrectly matching inside error_scope), and adds regression coverage to prevent reintroducing the bug.

Changes:

Anchor auth-param name matching to the start of the header or a parameter separator to avoid substring shadowing.
Escape field_name in the regex to ensure literal matching.
Add regression test cases covering both “shadowing” (real param present but a decoy exists) and “false positive” (only decoy param exists).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
`src/mcp/client/auth/utils.py`	Updates `extract_field_from_www_auth` regex to require a real auth-param name boundary and escape the field name.
`tests/client/test_auth.py`	Adds regression cases ensuring substring-shadowing and substring-only params no longer match.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

cubic-dev-ai

1 issue found across 2 files

_{Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic}

cubic-dev-ai

1 issue found across 2 files (changes from recent commits).

_{Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic}

cubic-dev-ai

2 issues found across 2 files (changes from recent commits).

_{Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic}

Whning0513 · 2026-07-06T18:12:08Z

Followed up on two additional parser edge cases. This now keeps the first Bearer challenge when multiple Bearer challenges appear in one WWW-Authenticate header, and it accepts auth-params written with optional whitespace around = (for example scope = "read write"). I added regression coverage for both cases and reran the focused auth tests locally.

fede-kamel

I differential-tested this against main and the two sibling PRs for #3009 (#3012, #3050) — full matrix in #3009 (comment).

Within Bearer challenges this behaves correctly in every probe I ran, including multi-challenge headers (Basic …, Bearer …) that regress in #3012. Two findings worth a look:

1. Silent contract change for non-Bearer lookups. Scoping extraction to the Bearer challenge is arguably the right semantics for the two current callers (scope per RFC 6750, resource_metadata per RFC 9728) — but extract_field_from_www_auth is a generic helper, and this changes its observable behavior:

extract_field_from_www_auth(r('Newauth realm="apps"'), "realm")
# main -> "apps";  this branch -> None

If Bearer-only is the intended direction, I would make it explicit (docstring at minimum, or fold the Bearer scoping into the two RFC-specific wrappers instead of the generic function) so the semantics are a reviewed decision rather than a side effect.

2. The quote-toggle splitter does not track escapes. _split_www_authenticate_segments flips in_quotes on every ", including RFC 9110 \" escapes, so an unpaired escaped quote before a comma still mis-splits the value (same limitation as main; #3012 handles this case). Since this PR already introduces a real parser, handling \" there would be a small addition that makes the new machinery strictly better than the regex it replaces.

Also flagging for the maintainers that #3050 fixes the same issue with a one-line boundary anchor and no semantic changes, in case the preference is to land the minimal fix first and layer challenge-scoping separately.

Whning0513 · 2026-07-19T19:14:02Z

Thanks for the detailed differential testing. I addressed both findings in d4cfbc5:

extract_field_from_www_auth keeps its generic non-Bearer behavior, while the RFC-specific scope and resource-metadata wrappers now select the first Bearer challenge.
The segment splitter now tracks quoted-pair escapes, and quoted auth-param values are parsed without treating escaped quotes or commas as delimiters.

I also added coverage for challenge boundaries, empty segments, empty quoted values, optional whitespace, and escaped quoted strings. The focused auth suite reports 143 passed and 1 xfailed; utils.py has 100% statement and branch coverage; Ruff and Pyright pass.

cubic-dev-ai

1 issue found across 2 files (changes from recent commits).

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/mcp/client/auth/utils.py">

<violation number="1" location="src/mcp/client/auth/utils.py:73">
P2: Generic auth-param lookup no longer recognizes valid parameter names such as `x.trace` or `client+id`, because the new name character class accepts only letters, digits, `_`, and `-`. Using the HTTP `token` character set here would preserve generic custom-scheme lookups.</violation>
</file>

_{Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic}

cubic-dev-ai · 2026-07-19T19:18:07Z

+
+
+_AUTH_PARAM_PATTERN = re.compile(
+    r"(?:^|,\s*)(?P<name>[A-Za-z][A-Za-z0-9_-]*)\s*=\s*"


P2: Generic auth-param lookup no longer recognizes valid parameter names such as x.trace or client+id, because the new name character class accepts only letters, digits, _, and -. Using the HTTP token character set here would preserve generic custom-scheme lookups.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/mcp/client/auth/utils.py, line 73: <comment>Generic auth-param lookup no longer recognizes valid parameter names such as `x.trace` or `client+id`, because the new name character class accepts only letters, digits, `_`, and `-`. Using the HTTP `token` character set here would preserve generic custom-scheme lookups.</comment> <file context> @@ -64,6 +69,35 @@ def _extract_bearer_auth_params(www_auth_header: str) -> str | None: +_AUTH_PARAM_PATTERN = re.compile( + r"(?:^|,\s*)(?P<name>[A-Za-z][A-Za-z0-9_-]*)\s*=\s*" + r'(?:"(?P<quoted>(?:\\.|[^"\\])*)"|(?P<unquoted>[^,\s]+))' +) </file context>

Suggested change

r"(?:^|,\s*)(?P<name>[A-Za-z][A-Za-z0-9_-]*)\s*=\s*"

r"(?:^|,\s*)(?P<name>[!#$%&'*+\-.^_`|~0-9A-Za-z]+)\s*=\s*"

Fix substring matches in WWW-Authenticate parsing

5de906e

Copilot AI review requested due to automatic review settings July 1, 2026 13:03

Copilot started reviewing on behalf of Whning0513 July 1, 2026 13:03 View session

Copilot AI reviewed Jul 1, 2026

View reviewed changes

cubic-dev-ai Bot reviewed Jul 1, 2026

View reviewed changes

Comment thread src/mcp/client/auth/utils.py Outdated

Avoid matching auth params inside quoted values

8a05814

cubic-dev-ai Bot reviewed Jul 1, 2026

View reviewed changes

Comment thread src/mcp/client/auth/utils.py Outdated

Handle Bearer auth params in multi-challenge headers

d725212

cubic-dev-ai Bot reviewed Jul 2, 2026

View reviewed changes

Comment thread src/mcp/client/auth/utils.py Outdated

Comment thread src/mcp/client/auth/utils.py

Handle more Bearer auth-param edge cases

3e99ffe

This was referenced Jul 10, 2026

WWW-Authenticate parsing matches a field name as a substring of another auth-param #3009

Open

fix: match complete www-authenticate auth params #3050

Open

fix(auth): match complete WWW-Authenticate parameters #3012

Open

fede-kamel reviewed Jul 10, 2026

View reviewed changes

Address WWW-Authenticate parser review feedback

d4cfbc5

cubic-dev-ai Bot reviewed Jul 19, 2026

View reviewed changes

Whning0513 added 2 commits July 20, 2026 03:24

Merge branch 'main' into fix-www-auth-substring-match-3009

b5df93c

Use current HTTP client name in auth tests

397896e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix substring matches in WWW-Authenticate parsing#3041

Fix substring matches in WWW-Authenticate parsing#3041
Whning0513 wants to merge 7 commits into
modelcontextprotocol:mainfrom
Whning0513:fix-www-auth-substring-match-3009

Whning0513 commented Jul 1, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

cubic-dev-ai Bot left a comment •

edited

Loading

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment •

edited

Loading

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Whning0513 commented Jul 6, 2026

Uh oh!

fede-kamel left a comment

Uh oh!

Whning0513 commented Jul 19, 2026

Uh oh!

cubic-dev-ai Bot left a comment

Uh oh!

cubic-dev-ai Bot Jul 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants



		_AUTH_PARAM_PATTERN = re.compile(
		r"(?:^\|,\s)(?P<name>[A-Za-z][A-Za-z0-9_-])\s=\s"

	r"(?:^\|,\s)(?P<name>[A-Za-z][A-Za-z0-9_-])\s=\s"
	r"(?:^\|,\s)(?P<name>[!#$%&'+\-.^_`\|~0-9A-Za-z]+)\s=\s"

Uh oh!

Conversation

Whning0513 commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Whning0513 commented Jul 6, 2026

Uh oh!

fede-kamel left a comment

Choose a reason for hiding this comment

Uh oh!

Whning0513 commented Jul 19, 2026

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Whning0513 commented Jul 1, 2026 •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading